Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: fix rpath issue #53

Merged
merged 1 commit into from
Apr 5, 2024
Merged

build: fix rpath issue #53

merged 1 commit into from
Apr 5, 2024

Conversation

chenrui333
Copy link
Contributor

While packaging for homebrew, I ran into some rpath issue when building the tools.

$ /opt/homebrew/Cellar/liblc3/1.1.0/bin/elc3 -h
dyld[72986]: Library not loaded: @rpath/liblc3.1.dylib
  Referenced from: <1B4E9174-0A9C-3907-8487-40BBBE5B52CB> /opt/homebrew/Cellar/liblc3/1.1.0/bin/elc3
  Reason: no LC_RPATH's found
Abort trap: 6

This patch is for fixing the rpath ref issue.

While packaging for homebrew, I ran into some rpath issue when building the tools.

```
$ /opt/homebrew/Cellar/liblc3/1.1.0/bin/elc3 -h
dyld[72986]: Library not loaded: @rpath/liblc3.1.dylib
  Referenced from: <1B4E9174-0A9C-3907-8487-40BBBE5B52CB> /opt/homebrew/Cellar/liblc3/1.1.0/bin/elc3
  Reason: no LC_RPATH's found
Abort trap: 6
```

This patch is for fixing the rpath ref issue.

Signed-off-by: Rui Chen <[email protected]>
@eli-schwartz
Copy link
Collaborator

Including an rpath pointing into the libdir is generally awkward when that is the default platform libdir. It is at best redundant, and at worst messes around with the load order. It overrides LD_LIBRARY_PATH.

How does homebrew usually handle rpaths?

@eli-schwartz
Copy link
Collaborator

This might help: mesonbuild/meson@78e9009

@chenrui333
Copy link
Contributor Author

How does homebrew usually handle rpaths?

usually we would just adopt the upstream build and dont care about it.

I do see for cmake builds, we did specify the rpath as well (which is quite similar in this case)

@antsoulier
Copy link

Sounds good for me.
@eli-schwartz Do you think we can move forward?

@eli-schwartz
Copy link
Collaborator

usually we would just adopt the upstream build and dont care about it.

If you don't care about it, then does that mean other meson packages in homebrew don't need this?

I do see for cmake builds, we did specify the rpath as well (which is quite similar in this case)

You specify the rpath how? Patching out CMakeLists.txt files?

@chenrui333
Copy link
Contributor Author

usually we would just adopt the upstream build and dont care about it.

If you don't care about it, then does that mean other meson packages in homebrew don't need this?

by "I dont care about it", meaning we adopt the upstream defaults (in this case, it certainly does not apply as it failed the tools build due to the rpath issue)

I do see for cmake builds, we did specify the rpath as well (which is quite similar in this case)

You specify the rpath how? Patching out CMakeLists.txt files?

yeah, patching PATH for cmake builds wrt rpath

@chenrui333
Copy link
Contributor Author

I dont quite comprehend the challenging of merging this PR (and it would certainly helpful for other people to do the meson builds for tools)

@asoulier asoulier merged commit 5e528fc into google:main Apr 5, 2024
7 checks passed
@asoulier
Copy link
Collaborator

asoulier commented Apr 5, 2024

Let's merge it, we can always modify it later.

@chenrui333
Copy link
Contributor Author

thanks @asoulier! appreciate it!

@carlocab
Copy link

carlocab commented Apr 9, 2024

How does homebrew usually handle rpaths?

We usually add the appropriate -Wl,-rpath flags to LDFLAGS1, but that doesn't seem to be respected here: Homebrew/homebrew-core#167734 (comment)

Not sure if this is due to a recent change in meson.

You specify the rpath how? Patching out CMakeLists.txt files?

We set CMAKE_INSTALL_RPATH when needed.

Footnotes

  1. Example: https://github.com/Homebrew/homebrew-core/blob/4951517b150814fd57ad3df98a88fb83d5a31fb5/Formula/lib/librist.rb#L35

@eli-schwartz
Copy link
Collaborator

Okay. In that case I think this is the answer:

This might help: mesonbuild/meson@78e9009

Normally the LDFLAGS are respected here. At least, it was tested to work on Linux. ;)

The problem is that Darwin took a different codepath... This was reported by Gentoo Prefix -- and also fixed by Gentoo Prefix. So I'm optimistic that the same fix will work for Homebrew as well. Can you try backporting it to the meson formula?

@chenrui333
Copy link
Contributor Author

The problem is that Darwin took a different codepath... This was reported by Gentoo Prefix -- and also fixed by Gentoo Prefix. So I'm optimistic that the same fix will work for Homebrew as well. Can you try backporting it to the meson formula?

maybe we can try in the 1.4.1 release? Looks like it would come pretty soon, right?

@chenrui333 chenrui333 deleted the fix-rpath-issue branch April 19, 2024 18:45
@asoulier
Copy link
Collaborator

will do this afternoon, I need to add a test case (HFP) for LC3 certification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants